Skip to content

migrate to scikit build system#22

Merged
chenpeizhi merged 3 commits intomasterfrom
scikit
Nov 13, 2025
Merged

migrate to scikit build system#22
chenpeizhi merged 3 commits intomasterfrom
scikit

Conversation

@gauravharsha
Copy link
Collaborator

@gauravharsha gauravharsha commented Nov 13, 2025

**Keeping C++ standard set at C++20/ But we should try to keep drudge and gristmill deps consistent.

Copilot AI review requested due to automatic review settings November 13, 2025 06:05
@coveralls
Copy link

coveralls commented Nov 13, 2025

Pull Request Test Coverage Report for Build 19322395186

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 91.198%

Totals Coverage Status
Change from base Build 19224772213: 0.0%
Covered Lines: 1782
Relevant Lines: 1954

💛 - Coveralls

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR migrates the build system from setuptools to scikit-build-core with CMake for building the C++ extension module. The migration removes the traditional setup.py approach and moves all configuration to pyproject.toml and a new CMakeLists.txt file.

Key changes:

  • Replaces setuptools build backend with scikit-build-core in pyproject.toml
  • Removes setup.py entirely, moving extension build configuration to CMake
  • Adds CMakeLists.txt with platform-specific compiler flags and C++ extension configuration

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
setup.py Removed entire setuptools-based build configuration
pyproject.toml Updated build system to use scikit-build-core and added CMake configuration settings
CMakeLists.txt New file defining C++ extension build with CMake, including include paths and platform flags

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@chenpeizhi
Copy link
Collaborator

I have started to update Drudge to C++20. In fact, I wanted to update libcanon to C++20 since concept, which is used in libcanon, is now standard in C++20; see DrudgeCAS/libcanon#5. However, this PR in libcanon modified the behavior of Drudge, which I still need to look into. I need to fix it or revert it.

@chenpeizhi chenpeizhi self-requested a review November 13, 2025 06:29
@gauravharsha
Copy link
Collaborator Author

I will also try to take a look at it next week and can help you out with the upgrades. Also, unrelated, but we used to have a pypi package for drudge. We should try to reconfigure one for drudge and gristmill. I have some experience with it and can try to look into it.

@chenpeizhi
Copy link
Collaborator

That would be great. Thanks!

For PyPI registration, I wonder whether it can be automated. Ideally, after each release, the bot can automatically register the package. Currently, I'm using release-please for automated releases. However, it does not work very well with the Python ecosystem. For example, it cannot automatically update the uv.lock file in a release PR. I'm considering switching to python-sematic-release; hopefully, it can handle automated uv lock and package registration.

@gauravharsha
Copy link
Collaborator Author

AFAIK, pypi packages can be automatically deployed, but it may end up creating a lot of versions. Instead, a better practice is to wait until sufficient changes are introduced and then release a new version/subversion.

@chenpeizhi
Copy link
Collaborator

Sounds good to me. We should probably do manual deployment for now. In the long run, we can consider continuous deployment for major releases only.

Copy link
Collaborator

@chenpeizhi chenpeizhi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks good to me. Thanks!

@chenpeizhi chenpeizhi merged commit 7316e49 into master Nov 13, 2025
2 checks passed
@gauravharsha gauravharsha deleted the scikit branch November 14, 2025 05:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants